Skip to content

chore(api-key): remove legacy scan+decrypt auth fallback#4876

Merged
TheodoreSpeaks merged 1 commit into
stagingfrom
feat/remove-fallback-api-key-check
Jun 4, 2026
Merged

chore(api-key): remove legacy scan+decrypt auth fallback#4876
TheodoreSpeaks merged 1 commit into
stagingfrom
feat/remove-fallback-api-key-check

Conversation

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator

Summary

  • Remove the legacy full-scan + decrypt fallback from API-key authentication — hash lookup is now the only path
  • Inline the authenticateApiKeyByHash helper into authenticateApiKeyFromHeader; a hash miss now returns invalid directly
  • Delete the now-orphaned authenticateApiKey decrypt-and-compare function and its safeCompare dependency
  • Update tests to drop the fallback-loop cases

Note

Keys whose key_hash column was never backfilled will now fail auth. Safe only if the backfill is complete and the fallback's API key matched via fallback decrypt loop warn count was zero in prod.

Type of Change

  • Chore / cleanup

Testing

Tested manually — bunx vitest run lib/api-key/ (46 tests pass), bun run lint, and bun run check:api-validation:strict all pass.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 4, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 4, 2026 1:37am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented Jun 4, 2026

PR Summary

High Risk
Changes authentication for all API traffic; keys missing key_hash will fail until backfill is complete.

Overview
API key authentication is now hash-only. authenticateApiKeyFromHeader looks up one row by sha256 of the header and applies scope, expiry, and permission checks; a miss or failed gate returns invalid. The legacy full-table scan + decrypt/compare fallback and the inlined authenticateApiKeyByHash helper are removed.

Removed authenticateApiKey from auth.ts (and its safeCompare usage), along with related unit tests for decrypt-based matching, lifecycle, and security edge cases. Service tests now expect a single DB where and no fallback warn logs.

Operational note: keys without a populated key_hash will no longer authenticate.

Reviewed by Cursor Bugbot for commit e4ed38a. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 4, 2026

Greptile Summary

This PR removes the legacy full-scan+decrypt fallback from authenticateApiKeyFromHeader, making hash-based lookup the only authentication path, and inlines the previously separate authenticateApiKeyByHash helper directly into the main function.

  • service.ts: Removes the legacy scan → decrypt → compare fallback loop; the previously private authenticateApiKeyByHash function is inlined directly into authenticateApiKeyFromHeader, trimming ~120 lines.
  • auth.ts: Deletes the now-orphaned authenticateApiKey (decrypt-and-compare) export and drops the safeCompare import — all callers already used authenticateApiKeyFromHeader.
  • Test files updated to remove fallback-path cases and stale authenticateApiKey mock wiring.

Confidence Score: 5/5

Safe to merge once the key_hash backfill in production is confirmed complete and the legacy warn log count is at zero — both conditions the PR author calls out explicitly.

The inlined logic is a direct lift of the previously tested fast path, no behavioral changes are introduced, all callers already used authenticateApiKeyFromHeader, and the deleted authenticateApiKey export has no remaining references in the codebase.

No files require special attention; all four changed files are straightforward deletions of dead code and their corresponding tests.

Important Files Changed

Filename Overview
apps/sim/lib/api-key/service.ts Inlines authenticateApiKeyByHash into authenticateApiKeyFromHeader and deletes the legacy fallback scan+decrypt loop; logic is functionally equivalent to the old fast path.
apps/sim/lib/api-key/auth.ts Removes authenticateApiKey export and safeCompare dependency; remaining functions (key creation, display formatting, encryption) are unchanged.
apps/sim/lib/api-key/service.test.ts Removes fallback-path test cases and mock wiring for authenticateApiKey; remaining tests correctly cover hash-lookup success, scope-check failure, and hash-miss.
apps/sim/lib/api-key/auth.test.ts Removes all test cases for the deleted authenticateApiKey function; remaining tests for format utilities, key generation, and encryption are intact.

Sequence Diagram

sequenceDiagram
    participant C as Caller
    participant S as authenticateApiKeyFromHeader
    participant DB as Database
    participant P as Permission Utils

    C->>S: apiKeyHeader, options
    alt no header
        S-->>C: "{success:false, error:"API key required"}"
    end
    alt options.workspaceId set
        S->>DB: getWorkspaceBillingSettings(workspaceId)
        DB-->>S: workspaceSettings (null → "Workspace not found")
    end
    S->>S: "keyHash = sha256(apiKeyHeader)"
    S->>DB: "SELECT WHERE key_hash = keyHash"
    DB-->>S: rows[]
    alt rows empty
        S-->>C: INVALID
    end
    S->>S: scope checks (userId, keyType, expiry, workspaceId)
    alt scope check fails
        S-->>C: INVALID
    end
    alt workspaceId + personal key
        S->>P: getUserEntityPermissions(userId, workspace, workspaceId)
        P-->>S: permission (null → INVALID)
    end
    S-->>C: "{success:true, userId, keyId, keyType, workspaceId}"
Loading

Reviews (1): Last reviewed commit: "chore(api-key): remove legacy scan+decry..." | Re-trigger Greptile

@TheodoreSpeaks TheodoreSpeaks merged commit 4076d76 into staging Jun 4, 2026
14 checks passed
@TheodoreSpeaks TheodoreSpeaks deleted the feat/remove-fallback-api-key-check branch June 4, 2026 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant